fix(python): handle PEP 440 direct references and path dependencies in uv export#490
Open
a-oren wants to merge 3 commits into
Open
fix(python): handle PEP 440 direct references and path dependencies in uv export#490a-oren wants to merge 3 commits into
a-oren wants to merge 3 commits into
Conversation
…n uv export Skip PEP 440 direct references (name @ url) and path dependencies (./, ../, /) in parseUvExport() with a log.fine() warning instead of throwing an IOException. These lines lack pinned versions needed for the dependency graph. Set currentKey to null after skipping to prevent # via comments from corrupting the graph of other packages. Implements TC-4538 Signed-off-by: Adva Oren <aoren@redhat.com> Assisted-by: Claude Code
Contributor
Reviewer's GuideUpdates PythonUvProvider.parseUvExport to gracefully skip PEP 440 direct references and path-based dependencies while preserving dependency graph integrity, and adds focused tests and fixtures to validate skip behavior and integration with SBOM generation (provideStack/provideComponent). Flow diagram for updated parseUvExport line handlingflowchart TD
A[parseUvExport line loop] --> B[check package line]
B --> C{trimmed contains @}
C -- yes --> D[log.fine Skipping PEP 440 direct reference]
D --> E[set currentKey null]
E --> A
C -- no --> F{trimmed startsWith ./, ../, or /}
F -- yes --> G[log.fine Skipping path dependency]
G --> H[set currentKey null]
H --> A
F -- no --> I{trimmed contains ==}
I -- no --> J[throw IOException]
I -- yes --> K[process pinned package]
K --> A
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The direct-reference and path-dependency detection is currently based on simple string checks (
trimmed.contains(" @ "),startsWith("./", "../", "/")); consider extracting these into clearly named helper methods with more precise matching (e.g., regex aligned with uv/PEP 440 formats) so that future edge cases (Windows paths,file:schemes,~/paths) can be handled consistently in one place. - The path-dependency skip logic only covers
./,../, and/prefixes; if uv can emit other local path forms (e.g.,~/local-packageor Windows-style paths), it might be worth adding explicit handling (or tests confirming they cannot appear) to avoid unexpected IOExceptions for those cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The direct-reference and path-dependency detection is currently based on simple string checks (`trimmed.contains(" @ ")`, `startsWith("./", "../", "/")`); consider extracting these into clearly named helper methods with more precise matching (e.g., regex aligned with uv/PEP 440 formats) so that future edge cases (Windows paths, `file:` schemes, `~/` paths) can be handled consistently in one place.
- The path-dependency skip logic only covers `./`, `../`, and `/` prefixes; if uv can emit other local path forms (e.g., `~/local-package` or Windows-style paths), it might be worth adding explicit handling (or tests confirming they cannot appear) to avoid unexpected IOExceptions for those cases.
## Individual Comments
### Comment 1
<location path="src/test/java/io/github/guacsec/trustifyda/providers/Python_Uv_Provider_Test.java" line_range="419-416" />
<code_context>
+ /** Verifies that path dependencies (./local-package) are skipped without throwing. */
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test to ensure `# via` comments after skipped path dependencies do not corrupt the graph
Please add a similar test case for path dependencies. For instance, cover a sequence like: normal package → path dependency + `# via` → normal package, and assert that `currentKey` is reset correctly for the path dep and that the `# via` block is not attached to the wrong package.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // anyio after the skipped line is still parsed correctly | ||
| assertThat(data.graph()).containsKey("anyio"); | ||
| assertThat(data.graph().get("anyio").version()).isEqualTo("3.6.2"); | ||
| assertThat(data.directDeps()).contains("anyio"); |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add a test to ensure # via comments after skipped path dependencies do not corrupt the graph
Please add a similar test case for path dependencies. For instance, cover a sequence like: normal package → path dependency + # via → normal package, and assert that currentKey is reset correctly for the path dep and that the # via block is not attached to the wrong package.
…, add ~/Windows path support Address code review feedback: - Extract isDirectReference() and isPathDependency() helper methods for clearer naming and a single place to extend edge case handling - Add ~/home-relative and Windows drive path (C:\) detection - Add test for # via graph corruption after skipped path dependencies Implements TC-4538 Signed-off-by: Adva Oren <aoren@redhat.com> Assisted-by: Claude Code
ruromero
approved these changes
May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
name @ url) and path dependencies (./,../,/) inparseUvExport()withlog.fine()warning instead of throwingIOExceptioncurrentKey = nullafter skipping to prevent# viacomments from corrupting the dependency graph of other packagesprovideStack()/provideComponent()Implements TC-4538
Test plan
parseUvExport()skips PEP 440 direct references without throwingparseUvExport()skips path dependencies (./,../,/) without throwing# viacomments after skipped packages do not corrupt the graphprovideStack()andprovideComponent()succeed with direct refs in export outputmvn spotless:checkpasses🤖 Generated with Claude Code
Summary by Sourcery
Handle uv export entries that lack pinned versions by skipping specific dependency types and expand test coverage to ensure dependency graphs and SBOM generation remain correct.
Bug Fixes:
Tests: